N°9144 Add data audit setup step by Timmy38 · Pull Request #801 · Combodo/iTop

Timmy38 · 2026-02-12T15:09:18Z

No description provided.

…ead of data audit, Catch exception during compilation to exit maintenance mode

Copilot

Pull request overview

This PR refactors the setup wizard into dedicated step classes and adds a new “data audit / compatibility check” step during upgrade, backed by a new step-based installation/audit sequencer.

Changes:

Split wizard step logic into individual setup/wizardsteps/* classes and update WizardController to use them (UpdateWizardStateAndGetNextStep).
Add a new upgrade-only wizard step (WizStepDataAudit) that runs a dry-run compilation + audit via DataAuditSequencer.
Introduce StepSequencer + ApplicationInstallSequencer infrastructure and update unattended install to use it.

Reviewed changes

Copilot reviewed 33 out of 35 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
setup/wizardsteps/WizardStep.php	New base class for wizard steps (+ dependency checking helper).
setup/wizardsteps/WizStepWelcome.php	New welcome step implementation.
setup/wizardsteps/WizStepInstallOrUpgrade.php	New install/upgrade selection step implementation.
setup/wizardsteps/WizStepDetectedInfo.php	New upgrade-detection step (chooses license vs misc params).
setup/wizardsteps/WizStepLicense.php	New license step (incl. GDPR consent gating).
setup/wizardsteps/WizStepLicense2.php	Upgrade-specific license step.
setup/wizardsteps/WizStepDBParams.php	New DB params step implementation.
setup/wizardsteps/WizStepAdminAccount.php	New admin account step implementation.
setup/wizardsteps/AbstractWizStepMiscParams.php	Shared misc params helpers (symlink + force-uninstall flags).
setup/wizardsteps/WizStepInstallMiscParams.php	New install misc params step (URL/graphviz/sample data).
setup/wizardsteps/WizStepUpgradeMiscParams.php	New upgrade misc params step (URL/graphviz + force-uninstall).
setup/wizardsteps/WizStepModulesChoice.php	Module/extension selection step; routes to DataAudit on upgrade.
setup/wizardsteps/WizStepDataAudit.php	New compatibility check step (runs audit sequencer before Summary).
setup/wizardsteps/WizStepSummary.php	New summary step (now also includes backup option on upgrade).
setup/wizardsteps/WizStepInstall.php	Install/upgrade execution step using sequencer + progress UI.
setup/wizardsteps/WizStepDone.php	Final step (backup download + hub iframe + enter iTop button).
setup/wizardcontroller.class.inc.php	Loads new step files, switches to `UpdateWizardStateAndGetNextStep`, and respects `CanComeBack()`.
setup/wizard.php	Removes legacy `wizardsteps.class.inc.php` include.
setup/ajax.dataloader.php	Removes legacy `wizardsteps.class.inc.php` include (relies on controller includes).
setup/unattended-install/unattended-install.php	Uses `ApplicationInstallSequencer` for unattended installs.
setup/unattended-install/InstallationFileService.php	Removes legacy `wizardsteps.class.inc.php` include.
setup/setup.js	Minor formatting changes to button update logic.
setup/sequencers/StepSequencer.php	New base sequencer with `ExecuteAllSteps()` loop.
setup/sequencers/ApplicationInstallSequencer.php	New main install/upgrade sequencer (split out of old installer).
setup/sequencers/DataAuditSequencer.php	New dry-run compile + audit + cleanup sequencer for compatibility checks.
setup/applicationinstaller.class.inc.php	Reduced to compatibility wrapper (`class_alias`) pointing to sequencer.
setup/SetupDBBackup.php	Moves `SetupDBBackup` out of old installer into its own file.
setup/runtimeenv.class.inc.php	Minor formatting fix.
setup/feature_removal/get_model_reflection.php	Disables MetaModel cache in reflection script.
setup/feature_removal/SetupAudit.php	Forces model reflection loading for both envs.
setup/feature_removal/ModelReflectionSerializer.php	Adjusts command invocation + error message.
css/setup.scss	Makes installation progress visible (removes forced `display:none`).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

setup/wizardsteps/WizStepSummary.php

setup/wizardcontroller.class.inc.php

setup/wizardsteps/WizStepDetectedInfo.php

setup/wizardsteps/WizStepModulesChoice.php

setup/wizardsteps/WizStepSummary.php

Copilot · 2026-02-12T17:35:19Z

setup/feature_removal/ModelReflectionSerializer.php

 		IssueLog::Info(__METHOD__, null, ['env' => $sEnv]);
 		$sPHPExec = trim(utils::GetConfig()->Get('php_path'));
 		$sOutput = "";
 		$iRes = 0;

-		exec(sprintf("$sPHPExec %s/get_model_reflection.php --env='%s'", __DIR__, $sEnv), $sOutput, $iRes);
+		$sCommandLine = sprintf("$sPHPExec %s/get_model_reflection.php --env=%s", __DIR__, $sEnv);
+		exec($sCommandLine, $sOutput, $iRes);
 		if ($iRes != 0) {
 			$this->LogErrorWithProperLogger("Cannot get classes", null, ['env' => $sEnv, 'code' => $iRes, "output" => $sOutput]);
-			throw new CoreException("Cannot get classes");
+			throw new CoreException("Cannot get classes from env ".$sEnv);
 		}


GetModelFromEnvironment() builds a shell command using $sPHPExec (from config) and $sEnv without shell escaping, then executes it via exec(). This is vulnerable to command injection if either value contains spaces or shell metacharacters. Build the command using escapeshellcmd() / escapeshellarg() (or better, bypass the shell entirely) before calling exec().

@Timmy38 not sure, but this could actually be a solution to the issue you had with quotes on Linux vs Windows.

setup/feature_removal/SetupAudit.php

Copilot · 2026-02-12T17:35:19Z

setup/wizardsteps/WizStepModulesChoice.php

+			}
+
+			if (isset($aChoice['sub_options'])) {
+				$aScores[$sChoiceId] = array_merge($aScores[$sChoiceId], $this->GuessDefaultsFromModules($aChoice['sub_options'], $aDefaults, $sChoiceId));
+			}


In GuessDefaultsFromModules(), the recursive call for sub_options passes $sChoiceId as the $aModules argument (GuessDefaultsFromModules(..., $sChoiceId)). This will break module lookup ($aModules[$sModuleId]...) and can cause runtime errors when a choice has sub_options. Pass the $aModules array and $sChoiceId as the parent id instead.

Hipska

I was wondering what exactly get's audited, it's not clear by reading the code.

I was also wondering if these audits are extendible by means of extensions.

Also, some suggestions that can picked up while doing this refactoring of wizard steps.

setup/wizardsteps/WizStepDataAudit.php

setup/feature_removal/SetupAudit.php

odain-cbd · 2026-02-14T20:27:45Z

setup/sequencers/DataAuditSequencer.php

+
+use Combodo\iTop\Setup\FeatureRemoval\SetupAudit;
+
+class DataAuditSequencer extends ApplicationInstallSequencer


overall remark. you add 2 sequencer. why not adding tests to make sure setup automata is what was designed. and to help the team see the sequence easily?

if we could test the setup sequence without executing all complex steps(mocking). it could nice.

A new StepSequencerTest class has been added and uses mock versions of both sequencers

setup/wizardcontroller.class.inc.php

odain-cbd · 2026-02-14T20:33:34Z

setup/wizardsteps/WizardStep.php

+	 * @param bool $bMoveForward True if the wizard is moving forward 'Next >>' button pressed, false otherwise
+	 * @return hash array('class' => $sNextClass, 'state' => $sNextState)
+	 */
+	abstract public function UpdateWizardStateAndGetNextStep($bMoveForward = true);


return an array is ok. an object would be nicer here. to type the 2 elements. WizardState for ex.

odain-cbd · 2026-02-14T20:36:00Z

setup/wizardcontroller.class.inc.php

-			$aNextStepInfo = $oStep->ProcessParams(true); // true => moving forward
+			$aNextStepInfo = $oStep->UpdateWizardStateAndGetNextStep(true); // true => moving forward
 			if (in_array($aNextStepInfo['class'], $aPossibleSteps)) {
 				$oNextStep = new $aNextStepInfo['class']($this, $aNextStepInfo['state']);


it could be nice to check the type of 'class' value before instanciation. also adding type.

A new method GetWizardStep check that the next step is a subclass of WizardStep

odain-cbd · 2026-02-14T20:37:19Z

setup/wizardcontroller.class.inc.php

+			if ($oStep->CanComeBack()) {
+				$this->PushStep(['class' => $sCurrentStepClass, 'state' => $sCurrentState]);
+			}
 			$aPossibleSteps = $oStep->GetPossibleSteps();


GetPossibleSteps seems useless to me. unless we keep it to test setup automata and use DumpXXX somewhere.

odain-cbd · 2026-02-14T20:38:07Z

setup/wizardcontroller.class.inc.php

@@ -169,7 +195,7 @@ protected function Back()
 		$sCurrentStepClass = utils::ReadParam('_class', $this->sInitialStepClass);
 		$sCurrentState = utils::ReadParam('_state', $this->sInitialState);
 		$oStep = new $sCurrentStepClass($this, $sCurrentState);


it could be nice to check the type before instanciation. also adding type afterwhile (@var XXX varname).

Same as above about GetWizardStep (with typing)

odain-cbd

lot of changes in setup wizard. but no test coverage of the controller/automata to make sure we are ok with sequence(s). it would be nice to add some test(s) and help the team figure out the sequence.
test would garantee the result and reduce end 2 end validation from a browser...

…mization, shellparameter escape

Timmy38 added 19 commits February 9, 2026 16:04

N°9144 WIP

85dd544

N°9144 Split wizard steps files

a54b11c

N°9144 Adding StepSequencer

fd6ce5f

N°9144 DataAuditSequencer v0

ea44e39

N°9144 Fix target env

4e769a1

N°9144 Move backup to recap screen

6b4413f

N°9144 Button Back from recap screen return to extensions choice inst…

d0ac19c

…ead of data audit, Catch exception during compilation to exit maintenance mode

N°9144 Add buttons "Go to backoffice" & "Ignore and continue"

e1c96ee

N°9144 Exit readonlymode when exception is thrown

709a278

N°9144 Fix CodeStyle

a2496f4

N°9144 Rebase Fix #1

b516466

N°9144 Rebase Fix #2

049a494

N°9144 Temp fix

f1c8591

N°9144 Remove temp flags

f5be709

N°9144 Remove temp flags

44d917c

N°9144 Fix unit tests

2f14b34

N°9144 Fix skip audit

7789d3e

N°9144 Code style

4d789ae

N°9144 Split audit & install sequencers

b257d25

Timmy38 requested review from odain-cbd and rquetiez February 12, 2026 15:09

Timmy38 self-assigned this Feb 12, 2026

CombodoApplicationsAccount added the internal Work made by Combodo label Feb 12, 2026

Molkobain requested a review from Copilot February 12, 2026 17:26

Molkobain added this to the 3.3.0 milestone Feb 12, 2026

Copilot started reviewing on behalf of Molkobain February 12, 2026 17:27 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

Hipska reviewed Feb 13, 2026

View reviewed changes

setup/wizardsteps/WizStepDataAudit.php Outdated Show resolved Hide resolved

setup/wizardsteps/WizStepDataAudit.php Outdated Show resolved Hide resolved

N°9144 Replace strings with class names

defca2d

odain-cbd reviewed Feb 14, 2026

View reviewed changes

setup/feature_removal/SetupAudit.php Show resolved Hide resolved

odain-cbd reviewed Feb 14, 2026

View reviewed changes

setup/wizardcontroller.class.inc.php Outdated Show resolved Hide resolved

odain-cbd reviewed Feb 14, 2026

View reviewed changes

odain-cbd requested changes Feb 14, 2026

View reviewed changes

Timmy38 added 5 commits February 17, 2026 10:04

N°9144 Check new step class before instanciating

0ace957

N°9144 CS fix

9875bae

N°9144 Sequencer unittest, WizardState object, MetaModel startup opti…

9cfeec8

…mization, shellparameter escape

N°9144 Rename NewStep

c64958d

N°9144 Fix Code Style

53cdb87

Timmy38 merged commit c4d7c89 into feature/uninstallation Feb 19, 2026

Timmy38 deleted the feature/9144_split_setup branch February 19, 2026 14:09


		use Combodo\iTop\Setup\FeatureRemoval\SetupAudit;

		class DataAuditSequencer extends ApplicationInstallSequencer

Conversation

Timmy38 commented Feb 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Hipska left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Timmy38 Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Timmy38 Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

odain-cbd left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Timmy38 Feb 19, 2026 •

edited

Loading

Timmy38 Feb 19, 2026 •

edited

Loading